Add Inlined React Runtime check for React 19 incompatibilities - #1380

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: gunjanjaswal <gunjanjaswal@git.wordpress.org>
Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org>
Co-authored-by: swissspidy <swissspidy@git.wordpress.org>
Co-authored-by: davidperezgar <davidperez@git.wordpress.org>
Co-authored-by: ernilambar <nilambar@git.wordpress.org>
Co-authored-by: jonathanbossenger <psykro@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Hello, I've got these findings from Codex. Could you check it?

Findings
[P1] Asset dependency suppresses a real inlined-runtime signal
Inlined_React_Runtime_Check.php (line 167) suppresses Symbol.for( 'react.element' ) whenever the sibling asset file contains react-jsx-runtime. But the marker is in the JS file being scanned; an asset dependency only proves the file declares an external dependency, not that it did not also inline a pre-19 runtime. This can miss the exact problem the check is meant to catch, especially with stale/manual asset files or mixed builds. I’d only use react-jsx-runtime in the asset file as supporting context, not as a bypass once the marker is present.

[P2] Removed API regex reports comments/strings as usage
Inlined_React_Runtime_Check.php (line 125) scans raw JS for unmountComponentAtNode, findDOMNode, and ReactCurrentOwner, so a comment, changelog string, translation string, or compatibility message will produce a warning. Since this is registered as a stable default check, that could create noisy false positives. Consider limiting this to property/member access patterns or parsing tokens enough to skip comments and string literals.

Thanks @davidperezgar, both are fair points. I pushed a fix for each.

P1 (asset dependency bypass): You're right that the .asset.php dependency only tells us the file declares react-jsx-runtime, not that it avoided inlining a pre-19 runtime. Since Symbol.for( 'react.element' ) is already a definitive inline marker, trusting the asset file there can hide the exact case the check is meant to catch. I dropped the asset file as a bypass, so now only an in-file window.ReactJSXRuntime reference suppresses the warning. I also added a fixture (asset-declared.js with its .asset.php) that inlines the runtime while declaring the dependency, and it's now flagged correctly.

P2 (comments and strings): Also right. The raw scan would flag unmountComponentAtNode, findDOMNode, or ReactCurrentOwner even inside a comment, changelog line, or translation string. It now blanks out comments and string literals before scanning, keeping the length and newlines intact so the reported line and column stay accurate, and only real usages get flagged. I added a comment-only.js fixture that mentions all three in prose and stays clean.

Happy to adjust either if you'd prefer a different approach.

Bumping this gently. The Inlined React Runtime check has been green for a few weeks now and still mergeable. If anything on trunk has moved under it I'm glad to rebase; otherwise it's ready for a review pass whenever the team has bandwidth.

Thanks @jsnajdr, and thanks again for strengthening the detection before this landed. Glad it lines up with the WP 7.2 React 19 rollout, that's exactly the kind of breakage this is meant to catch early. Happy to help if anything else comes up as you start testing plugins against it.

Reviewed by AI: Opus 5.5

Summary

This PR adds a react_usage static check. It reports errors when a JS file inlines a pre-19 react/jsx-runtime, react or react-dom, and warnings when it calls React APIs that React 19 removed. The primary signal ("react.element" plus a per-package fingerprint) covers the issue's headline case well. However, the package fingerprints are loose enough to produce false errors in a stable, default-on check, and some messages overstate what actually breaks on WordPress.

✅ What's good

• Two-step detection (element marker + package-specific fingerprint) is a real improvement over the issue's bare Symbol.for("react.element") grep. react-is no longer triggers it (react-is.js fixture).
• Matching the string literal rather than Symbol.for(...) catches React 17 production builds that hoist Symbol.for into a local (react-17-prod.js).
• The *.asset.php bypass was dropped as requested in the P1 feedback. asset-declared.js locks that in.
• The per-package codes (inlined_react_jsx_runtime, inlined_react, inlined_react_dom) give actionable output. Detecting development builds is a nice touch.
• Blanking comments and strings before the removed-API scan preserves offsets, so line and column stay correct.
• The fixtures cover mixed builds well: tree-shaken jsxs, and window.ReactDOM not suppressing an inlined react.
• PHPCS passes on the new files. All CI jobs are green (PHP 7.4–8.4, WP 6.3/latest/trunk, PHPStan, sniffs).

⚠️ Must-fix before merge

• includes/Checker/Checks/Performance/React_Usage_Check.php:234 — The react/jsx-runtime fingerprint /\bjsxs?\s*[:=][^=]/ matches any jsx key or assignment in the file. Any bundle that contains "react.element" for an unrelated reason (e.g. react-is pulled in via prop-types / hoist-non-react-statics, which is very common) gets an error if it also has:

  • Prism.languages.jsx = …
  • { ecmaFeatures: { jsx: true } }
  • a highlight.js or CodeMirror jsx: alias
  • any local const jsx = … that isn't literally window.ReactJSXRuntime

  I confirmed this with synthetic inputs: Symbol.for("react.element") together with Prism.languages.jsx = … or {jsx:true} is reported as inlined_react_jsx_runtime. The fingerprint needs to be tied to the runtime itself. For example, require the jsx assignment on an exports-like object, or co-occurrence with the runtime's ref/key/_owner element literal or its "react.fragment" export.

• includes/Checker/Checks/Performance/React_Usage_Check.php:244 — The react fingerprint also matches preact/compat, which defines both Symbol.for('react.element') and __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED = {…}. A Preact plugin never touches WordPress's React, yet it would get an error saying it "breaks when WordPress upgrades to React 19".

• includes/Checker/Checks/Performance/React_Usage_Check.php:188-208 — The errors claim every inlined package "breaks when WordPress upgrades to React 19". That is only true when elements cross the boundary: an inlined runtime or react feeding WordPress's react-dom / @wordpress/components, or the reverse. A fully self-contained React 18 bundle (runtime + react + react-dom all inlined, rendering its own root) gets three errors but keeps working. It's bloat, not breakage. Also, the issue proposed warning severity ("detection is heuristic"), and the PR description still says "Everything's a warning, nothing errors out". The PR now emits errors at severity 6/7. Pick one, and make it explicit:

  • (a) Error only for mixed builds, where at least one of the three packages is inlined and another is externalized; warning otherwise.
  • (b) Warning across the board.

  Either way, the message text must match what actually happens.

• includes/Checker/Checks/Performance/React_Usage_Check.php:302 — The Gutenberg React 19 work (WordPress/gutenberg#78899, merged) polyfills render, hydrate and unmountComponentAtNode in @wordpress/element and in the react-dom script. A plugin that calls ReactDOM.render/hydrate/unmountComponentAtNode on WordPress's externalized react-dom does not "stop working once WordPress upgrades React". The warning is false for those three. Either reword it for them (deprecated, polyfilled, migrate to createRoot/hydrateRoot/root.unmount()) or give them a separate, softer message. findDOMNode, unstable_renderSubtreeIntoContainer, renderToNodeStream and createFactory are not polyfilled and can keep the current wording.

• includes/Checker/Checks/Performance/React_Usage_Check.php:352, :367 — The replacement values 'a ref on the element' and 'JSX or createElement()' are English prose substituted into a translated sentence via %2$s, so they can never be translated. Pass only code identifiers through the placeholder (createRoot(), hydrateRoot(), …). For the two prose cases, use a dedicated translatable string, or wrap the prose in __().

• readme.txt:109 — The changelog entry is added under = 2.0.0 =, but 2.0.0 and 2.1.0 are already tagged and released (plugin.php is at 2.1.0). Move it to the next release section, or leave it for the release PR.

• includes/Checker/Checks/Performance/React_Usage_Check.php:33 (and every @since 2.0.0 in the file) — Same problem. This code ships in the next release, not 2.0.0. Update @since to the next version.

💡 Suggestions (optional)

• includes/Checker/Checks/Performance/React_Usage_Check.php:239 — Global suppression treats any window.React reference as proof of externalization. A bundle that inlines React and exposes it with window.React = exports (a common pattern) is therefore silently cleared. I confirmed this false negative. Consider matching only read positions, e.g. rejecting window.React\s*=(?!=).

• includes/Checker/Checks/Performance/React_Usage_Check.php:229 — Unminified webpack output emits externals as window["ReactJSXRuntime"] / window["React"]. Terser only rewrites them to dot access when minifying. Accepting window\[\s*(['"])ReactJSXRuntime\1\s*\] makes suppression work for development builds too.

• includes/Checker/Checks/Performance/React_Usage_Check.php:386 — The comment/string blanker has two weaknesses:

  • It does not handle regex literals. var r=/"/g;findDOMNode(e);var s="x"; swallows the findDOMNode call, a false negative I confirmed.
  • On large vendor bundles with long string literals, PCRE hits its recursion limit. On a 3 MB literal it returned null and fell back to the raw contents, which silently brings back the comment/string false positives that P2 fixed.

  Either document this as an accepted limitation, or skip blanking above a size threshold and report nothing for the removed APIs in that file.

• includes/Checker/Checks/Performance/React_Usage_Check.php:66 — CATEGORY_PERFORMANCE is an odd home for what is mainly a compatibility check. Only the development-build note is performance-related. CATEGORY_GENERAL fits better, unless the team wants a compatibility bucket (issue open question 3).

• includes/Checker/Checks/Performance/React_Usage_Check.php:46 — The issue asked for a link to the React 19 upgrade post on make.wordpress.org. That is more relevant to WordPress plugin authors than react.dev's generic guide, especially for the inlined_* errors.

• includes/Checker/Checks/Performance/React_Usage_Check.php:93 — Skipping the removed-API scan for any file with an inlined package also hides the plugin's own findDOMNode calls when only the JSX runtime was inlined. That's acceptable, but worth a line in the docblock since the comment only justifies the react-dom case.

• tests/phpunit/tests/Checker/Checks/React_Usage_Check_Tests.php — Add negative fixtures for the false-positive shapes above: react-is + jsx: key, preact/compat, and a window["ReactJSXRuntime"] dev build. There is also no fixture proving that window.React / window.ReactDOM suppress their own packages.

• PR description — It is stale: it still describes inlined_react_runtime, Inlined_React_Runtime_Check_Tests, inlined_jsx_runtime, ReactCurrentOwner detection, *.asset.php suppression and warnings only. Update it so the merge commit and changelog reflect what ships.

Verdict

Request Changes: the detection idea is sound, but a stable, default-on check emitting errors needs tighter fingerprints (JSX-key and preact/compat false positives) and messages that match actual React 19 behavior on WordPress (self-contained bundles, polyfilled render/hydrate/unmountComponentAtNode), plus the version and i18n fixes.